Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata #8171

hannesrudolph · 2025-09-18T23:55:18Z

Closes #8153

Before:

FocuSee.Project.2025-09-19.18-07-19.mp4

AFTER

FocuSee.Project.2025-09-19.18-12-22.mp4

Problem

Users occasionally saw a duplicate/overlaid task after pressing “Cancel” or after a mid-stream network failure—most visibly during “reasoning” streaming.
Root cause: two independent rehydration paths racing:
- Task-level catch during the streaming loop rehydrated from history
  Task.recursivelyMakeClineRequests()
- Provider cancel path also rehydrated from history
  ClineProvider.cancelTask()
This race was easier to hit during reasoning because reasoning chunks are UI-only (persisted to clineMessages) and not added to apiConversationHistory mid-stream, so the Task catch reached rehydrate faster.

Changes

Centralize rehydration in the Provider
- Removed Task-side rehydrate in the streaming error/cancel catch; the Task now:
  - Persists an interruption marker first
  - Sets an explicit abortReason
  - Calls abortTask() and emits TaskAborted
  - Does not call createTaskWithHistoryItem
- Provider listens for TaskAborted and rehydrates only for streaming failures (not user-cancels)
  Task.recursivelyMakeClineRequests()
  ClineProvider.onTaskAborted
Prevent double-create by marking “abandoned” earlier on user-cancel
- Set task.abortReason = "user_cancelled" and mark the current Task instance abandoned immediately after abortTask(), before waiting for the stream to drain
  ClineProvider.cancelTask()
- This ensures the Task catch never tries to rehydrate.
Provider-side cancel bookkeeping to mirror abortStream for user cancels
- Because setting abandoned early can skip abortStream on user-cancel, the Provider now fills the same persistence gaps:
  - Update the last say("api_req_started") UI row with cancelReason = "user_cancelled"
    ClineProvider.cancelTask()
  - Append an assistant “Response interrupted by user” marker to apiConversationHistory when appropriate
    ClineProvider.cancelTask()
- This preserves UI/API consistency and cancellation telemetry without reintroducing the race.
Resume reconciliation for UI-only reasoning
- On resuming from history, remove trailing reasoning-only UI rows that never entered apiConversationHistory (orphan partials)
  Task.resumeTaskFromHistory()
Defensive safeguards to avoid rehydrate after rehydrate
- Before rehydrating on TaskAborted (streaming_failed) or after cancel wait, check if the current instance already changed; skip if so
  ClineProvider.onTaskAborted
  ClineProvider.cancelTask()

Why this approach

Eliminates the duplicate/overlay by making the Provider the single source of rehydration truth.
Maintains a coherent timeline when cancelling mid-reasoning by explicitly persisting:
- cancelReason into the last api_req_started row
- an assistant “interrupted by user” entry into API history
Keeps normal success flows unchanged and produces a clean resume prompt consistently.

Behavioral impact

Cancel during reasoning: one rehydrate, no duplicates; cancelReason and interruption marker preserved.
Network failure mid-stream: Task emits TaskAborted with abortReason="streaming_failed"; Provider rehydrates once.
Cancel during text streaming: one rehydrate; no duplicates.
didFinishAbortingStream may not set on user-cancel (due to early abandoned), but Provider waits on other safe conditions and applies the same persistence updates Provider-side.

Key references

Removed Task-side rehydrate:
Task.recursivelyMakeClineRequests()
Provider-owned rehydrate and cancel bookkeeping:
ClineProvider.onTaskAborted
ClineProvider.cancelTask()
Resume reasoning reconciliation:
Task.resumeTaskFromHistory()

Important

Centralizes task rehydration in ClineProvider.ts to prevent duplicate rehydration during reasoning and updates localization for interruption messages.

Behavior:
- Centralizes rehydration in ClineProvider.ts, removing task-side rehydration in Task.ts.
- ClineProvider.ts now handles rehydration only for streaming failures, not user cancels.
- Marks tasks as "abandoned" earlier on user-cancel to prevent double rehydration.
- Updates UI/API consistency by appending interruption markers to apiConversationHistory.
Files:
- Task.ts: Removes task-side rehydration logic, adds abortReason property.
- ClineProvider.ts: Implements centralized rehydration logic, updates cancel bookkeeping.
- Localization files: Adds interruption messages for user and API error interruptions.
Misc:
- Ensures no rehydration occurs if the task instance has already changed.
- Improves handling of reasoning-only UI rows during task resumption.

^{This description was created by}^{for b1958e8. You can customize this summary. It will automatically update as commits are pushed.}

…ze rehydrate in provider and preserve cancel metadata - Remove Task-side rehydrate on stream error/cancel; provider owns rehydrate - Set abandoned early on user cancel to prevent Task catch from rehydrating - Provider writes cancelReason to last api_req_started and appends assistant interruption - Trim orphan reasoning-only UI rows on resume - Add defensive guards to avoid rehydrate-after-rehydrate

Copilot

Pull Request Overview

Centralizes task rehydration logic to prevent duplicate rehydrations during reasoning interruptions and preserves cancellation metadata. The changes move rehydration control from the Task class to the ClineProvider to avoid race conditions.

Centralizes rehydration logic in ClineProvider with defensive guards against duplicate operations
Preserves cancel metadata in both UI and API message histories during user-initiated cancellations
Removes trailing reasoning-only UI messages during task rehydration to maintain consistency

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
ClineProvider.ts	Centralizes rehydration in onTaskAborted handler with defensive instance checks, adds comprehensive cancel bookkeeping
Task.ts	Adds abortReason tracking, removes local rehydration logic, cleans up trailing reasoning messages during rehydration

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-18T23:55:51Z

src/core/webview/ClineProvider.ts

+				try {
+					// Only rehydrate on genuine streaming failures.
+					// User-initiated cancels are handled by cancelTask().
+					if ((instance as any).abortReason === "streaming_failed") {


Using type assertion (instance as any) bypasses type safety. Consider adding abortReason to the Task interface or creating a proper type guard to check for this property safely.

Copilot · 2025-09-18T23:55:51Z

src/core/webview/ClineProvider.ts

 		const parentTask = task.parentTask

+		// Mark this as a user-initiated cancellation so provider-only rehydration can occur
+		task.abortReason = "user_cancelled"


Direct property assignment to abortReason suggests this property may not be part of the Task interface. Consider adding this property to the Task interface or using a setter method to ensure type safety.

ellipsis-dev · 2025-09-18T23:57:55Z

src/core/webview/ClineProvider.ts

+			// Update ui_messages: add cancelReason to last api_req_started
+			const messagesJson = await fs.readFile(uiMessagesFilePath, "utf8").catch(() => undefined)
+			if (messagesJson) {
+				const uiMsgs = JSON.parse(messagesJson) as ClineMessage[]


Wrap the JSON.parse call on uiMessagesFilePath content in a try/catch to handle potential parsing errors gracefully.

^{This comment was generated because it violated a code review rule: irule_PTI8rjtnhwrWq6jS.}

ellipsis-dev · 2025-09-18T23:57:55Z

src/core/webview/ClineProvider.ts

+
+		// Provider-side cancel bookkeeping to mirror abortStream effects for user_cancelled
+		try {
+			// Update ui_messages: add cancelReason to last api_req_started


Refactor the cancelTask method to extract UI and API history updating logic into smaller helper functions for improved readability and maintainability.

roomote

Thank you for this comprehensive fix to the duplicate rehydration issue! The centralization of rehydration logic in the Provider and the defensive guards against race conditions are solid architectural improvements.

Review Summary

I've reviewed the changes and found that the previously raised type safety concerns have been addressed - abortReason is now properly defined in the Task class.

Suggestions for Further Improvement

Type assertion could be avoided - In ClineProvider.ts, the code uses (instance as any).abortReason even though abortReason is properly defined in the Task class. Consider using proper typing instead.
Potential infinite loop in reasoning cleanup - The while loop in Task.ts that removes trailing reasoning messages could theoretically run indefinitely if messages are malformed. Consider adding a safety limit.
Duplicate cancel bookkeeping logic - The cancel bookkeeping logic appears in both ClineProvider.cancelTask() and Task.abortStream(). Consider consolidating to reduce duplication.
Extract magic strings as constants - Strings like "streaming_failed", "user_cancelled", and "[Response interrupted by user]" appear multiple times. Would be cleaner as named constants.
Enhanced logging - Consider adding success logs when rehydration is performed, not just when it's skipped.

Overall, this is a well-thought-out solution that properly addresses the race condition issue. The changes effectively prevent duplicate rehydration and maintain proper state consistency.

- Mark original task instance abandoned instead of getCurrentTask() - Add parse guards and centralize UI/API cancel bookkeeping helpers - Replace any-cast with typed instance.abortReason - Centralize interruption strings and reuse in Task/Provider - Add final instanceId race-check before rehydrate - Remove unused variable in cancelTask()

hannesrudolph · 2025-09-19T00:30:25Z

Summary of applied fixes (items 1–6)

Implemented

Mark the original instance as abandoned (not getCurrentTask)

Updated to set the concrete task’s flag, avoiding incorrect instance marking.
See ClineProvider.cancelTask()

Guard JSON parsing and centralize cancel bookkeeping

Replaced ad‑hoc fs/JSON.parse with safe helpers that read/write through the persistence layer.
New helpers:
- addCancelReasonToLastApiReqStarted
- appendAssistantInterruptionIfNeeded
Call sites:
- ClineProvider.cancelTask()

Final race re-check before rehydrate

Added a second instanceId equality check immediately before rehydration to avoid duplicate rehydrate.
See ClineProvider.cancelTask()

Remove any-cast for abortReason

Use strongly-typed property on Task instance instead of (instance as any).abortReason.
See ClineProvider.onTaskAborted

Centralize user-facing interruption strings (prep for i18n)

New constants: messages.ts
Replaced hard-coded strings:
- Task.recursivelyMakeClineRequests() → abortStream label usage
- ClineProvider.cancelTask() assistant interruption

Deduplicate cancel bookkeeping logic

Extracted shared logic into persistence helpers to keep a single source of truth.
See cancelBookkeeping.ts

Additional cleanup

Removed unused destructured variable (apiConversationHistoryFilePath).
- ClineProvider.cancelTask()

Notes

The ‘reasoning cleanup’ hard-cap suggestion was not implemented (only items 1–6 were requested). The existing loop already terminates on first non-reasoning row and is linear in length; adding a cap remains possible if desired.

CI

Pushed changes to branch fix/reasoning-cancel-centralized-rehydrate. Checks are now pending.

daniel-lxs · 2025-09-19T16:00:44Z

src/shared/messages.ts

+ *
+ * Note: These are plain phrases (no surrounding brackets). Call sites add any desired decoration.
+ */
+export const RESPONSE_INTERRUPTED_BY_USER = "Response interrupted by user"


These need to be internationalized

daniel-lxs · 2025-09-19T16:15:26Z

src/core/task-persistence/cancelBookkeeping.ts

+
+		try {
+			const existing = uiMsgs[idx]?.text ? JSON.parse(uiMsgs[idx].text as string) : {}
+			uiMsgs[idx].text = JSON.stringify({ ...existing, cancelReason: reason })


Should we check if there's already a reason here before overwriting?

…lized strings and remove unused message constants

…iting - Add conditional check to preserve existing cancelReason values - Only set cancelReason if it doesn't already exist - Addresses review feedback from daniel-lxs

- Delete cancelBookkeeping.ts entirely - Remove bookkeeping imports and calls from ClineProvider - Simplify abortStream in Task.ts - Keep core fixes: centralized rehydration, abortReason tracking, abandoned flag - Let UI handle cancellation display without modifying persisted data

- Update log messages to reflect that we're no longer doing bookkeeping - Rename variable from currentAfterBookkeeping to currentAfterCheck - Make the code accurately describe what it's doing (race condition checks)

hannesrudolph requested review from cte, jr and mrubens as code owners September 18, 2025 23:55

Copilot AI review requested due to automatic review settings September 18, 2025 23:55

github-project-automation bot added this to Roo Code Roadmap and Roo Code Roadmap Sep 18, 2025

github-project-automation bot moved this to Triage in Roo Code Roadmap Sep 18, 2025

github-project-automation bot moved this to New in Roo Code Roadmap Sep 18, 2025

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Sep 18, 2025

Copilot AI reviewed Sep 18, 2025

View reviewed changes

ellipsis-dev bot reviewed Sep 18, 2025

View reviewed changes

roomote bot reviewed Sep 19, 2025

View reviewed changes

hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 19, 2025

hannesrudolph moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 19, 2025

hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 19, 2025

hannesrudolph force-pushed the fix/reasoning-cancel-centralized-rehydrate branch from d65a8f5 to 3bbf984 Compare September 19, 2025 01:09

daniel-lxs reviewed Sep 19, 2025

View reviewed changes

daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Sep 19, 2025

refactor(interruption): replace hardcoded response messages with loca…

e5be214

…lized strings and remove unused message constants

hannesrudolph added PR - Changes Requested and removed PR - Needs Preliminary Review labels Sep 19, 2025

daniel-lxs added 3 commits September 19, 2025 11:37

fix(cancelBookkeeping): check for existing cancelReason before overwr…

054764c

…iting - Add conditional check to preserve existing cancelReason values - Only set cancelReason if it doesn't already exist - Addresses review feedback from daniel-lxs

fix: remove misleading bookkeeping references from log messages

b1958e8

- Update log messages to reflect that we're no longer doing bookkeeping - Rename variable from currentAfterBookkeeping to currentAfterCheck - Make the code accurately describe what it's doing (race condition checks)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata #8171

Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata #8171

hannesrudolph commented Sep 18, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 18, 2025

Uh oh!

Copilot AI Sep 18, 2025

Uh oh!

ellipsis-dev bot Sep 18, 2025

Uh oh!

ellipsis-dev bot Sep 18, 2025

Uh oh!

roomote bot left a comment

Uh oh!

hannesrudolph commented Sep 19, 2025

Uh oh!

daniel-lxs Sep 19, 2025

Uh oh!

daniel-lxs Sep 19, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata #8171

Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata #8171

Conversation

hannesrudolph commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

roomote bot left a comment

Choose a reason for hiding this comment

Review Summary

Suggestions for Further Improvement

Uh oh!

hannesrudolph commented Sep 19, 2025

Uh oh!

daniel-lxs Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-lxs Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hannesrudolph commented Sep 18, 2025 •

edited

Loading

daniel-lxs Sep 19, 2025 •

edited

Loading